feat: add stale-on-error Redis recovery - #121
Draft
lan17 wants to merge 1 commit into
Draft
Conversation
This was referenced Aug 2, 2026
lan17
added a commit
that referenced
this pull request
Aug 7, 2026
## Summary Replace read-side Lua with native Redis commands and decode DialCache's frame in TypeScript: - untracked reads use `GET` - tracked reads use one atomic, primary-routed `MGET` for the value and watermark - write and invalidation remain Lua-backed; a watermark-fenced tracked write now atomically unlinks the stale value it rejects - node-redis registers only the three mutation scripts, and GLIDE owns only the three mutation script handles - custom adapters can reuse the public `decodeRedisFrame` and `decodeTrackedRedisFrame` helpers This removes the Redis-to-Lua payload materialization and `string.sub` copy on every hit while preserving the semantic `DialCacheRedisClient.read()` boundary. ## Read architecture | Adapter / mode | Untracked | Tracked | Primary guarantee | | --- | --- | --- | --- | | node-redis standalone | `GET` | `MGET` | standalone connection | | node-redis Cluster | `GET` | raw `MGET` | `sendCommand(..., false, ...)` routes to the slot primary | | GLIDE standalone | `GET` | one-command `Batch(false).mget(...)` | standalone batches execute on the primary even with replica reads configured; `MGET` itself is atomic | | GLIDE Cluster | `GET` | custom-command `MGET` | explicit `primarySlotKey` route | The shared decoder: - validates the frame version and minimum length - preserves missing/short/unsupported frames as clean misses - parses integer and fractional legacy watermarks with the same accepted grammar as Lua - rejects values whose Redis-created timestamp is at or before the watermark - preserves unsupported payload encodings as `DialCacheRedisPayloadEncodingError` - returns binary payloads through a zero-copy `Buffer.subarray()` view Tracked value and watermark reads retain one atomic snapshot, with both values returned by a single `MGET`. Their existing shared Cluster hash tag remains required; mismatched tags still fail with `CROSSSLOT`. ## Breaking change - `READ_CACHE_SCRIPT` and `READ_TRACKED_CACHE_SCRIPT` are removed from `dialcache/redis-protocol`. - `dialcacheRedisScripts.dialcacheRead` and `dialcacheRedisScripts.dialcacheReadTracked` are removed from `dialcache/node-redis`. - Custom node-redis wrappers must expose native `get` / `sendCommand`; `legacyMode` clients are unsupported because neither their callback surface nor `.v4` view exposes the complete native-command-plus-custom-script contract. - The GLIDE helper requires GLIDE 2.x, a direct official `GlideClient` or `GlideClusterClient`, and the same module namespace that created it. Forwarding wrappers should implement `DialCacheRedisClient` directly because their topology cannot be inferred safely. - Official node-redis clients and direct GLIDE 2.x clients passed through the documented helpers keep the same application-facing call shape, so those consumers can bump the package without code changes. - Redis keys, frame format, and invalidation behavior are unchanged. A tracked write rejected by an active future watermark still returns `false`, but now also unlinks the stale value key. No data migration or cache flush is required. - The fenced-write cleanup requires `UNLINK` (Redis 4.0+ or compatible Valkey) and permission for scripts to invoke it. With a command-restricted ACL that denies `UNLINK`, the write fails open as `cache_write` and leaves the stale value for a later cleanup or expiry. `BREAKING CHANGE:` the four deprecated read-Lua exports and registrations above are removed; node-redis adapters require the promise-mode native-command surface; the GLIDE helper requires a direct GLIDE 2.x client from the supplied runtime; and the fenced-write cleanup requires Redis `UNLINK` support plus ACL permission. Under the repository's release configuration, this change should release as `v1.0.0`. ## Adapter behavior changes - The node-redis factory now requires native `get` and `sendCommand` methods in addition to the three registered mutation methods. - The GLIDE factory declares an optional `@valkey/valkey-glide ^2.0.0` peer, validates `Batch` support eagerly, and classifies standalone versus cluster behavior from the supplied runtime's client identities before allocating scripts. Its standalone non-atomic primary batch avoids consuming caller-owned `WATCH` state. - Redis `MGET` returns `null` for wrong-type members. A tracked wrong-type value is therefore a clean miss and may be repaired with a valid DialCache frame after fallback succeeds, while a wrong-type watermark prevents the tracked write from succeeding. An untracked `GET` still surfaces `WRONGTYPE`. Real-engine tests cover both repair and repeated fail-open behavior, including metrics. - The public read contract now specifies frame decoding, miss and watermark rules, atomic authoritative snapshots, and returned-buffer ownership. Shared decoders validate leaf reply types; adapters retain only client-specific envelope validation. ## Benchmark The benchmark harness and JSON results were intentionally kept outside the repository. Methodology: - Redis 6.2.22 and Valkey 8.1.8 - Node 22.22.0, node-redis 4.7.1, GLIDE 2.4.2 - binary payloads of 100 B, 1 KiB, 10 KiB, 100 KiB, and 1 MiB - fresh untracked hit, fresh tracked hit, and invalidated tracked miss - three alternating rounds, one command in flight, loopback Docker - median throughput, latency, Redis `INFO commandstats` execution time, and network bytes At 1 MiB, native fresh-hit throughput improved 15-45% across the two engines and adapters. Server-reported command execution time per logical read fell 95-98%. Small 100 B / 1 KiB end-to-end results were mostly flat/noisy while reported command time still fell about 80-90%; the notable small-case regression was Redis/node-redis's 100 B tracked hit at about -10% throughput. These loopback, one-in-flight results are directional rather than production-capacity measurements. Representative Redis 6.2 + node-redis medians: | 1 MiB scenario | Lua ops/s | Native ops/s | Lua server us/read | Native server us/read | Lua -> native p50 | | --- | ---: | ---: | ---: | ---: | ---: | | untracked hit | 230 | 269 | 719.8 | 32.6 | 3.718 ms -> 2.955 ms | | tracked hit | 217 | 259 | 713.7 | 31.2 | 3.630 ms -> 3.016 ms | | invalidated tracked miss | 1,762 | 284 | 361.6 | 31.0 | 0.566 ms -> 2.949 ms | The invalidated-miss row is the main tradeoff: Lua returns only a null reply, while native `MGET` transfers the stale frame before TypeScript rejects it. At 1 MiB this changes roughly 3-5 response bytes into about 1.05 MB. Across both engines and adapters, invalidated-miss throughput fell 77-84% at 1 MiB (46-58% at 100 KiB), even though server-reported command time still fell 91-94%. The benchmark intentionally measured the read itself and therefore includes that full transfer. In the application path, the first completed fallback that reaches a still-fenced tracked write now atomically unlinks the stale value, bounding subsequent transfers for that entry. This is only a partial mitigation: a read failure or timeout never reaches the write-side cleanup, so the stale payload can continue to transfer or time out until another completed read cleans it up or its TTL expires. ## Scope This branch is updated onto the current `v0.15.0` read contract, including the untracked-cache shadowing changes from #122. It deliberately does not include the server-time / maximum-age behavior proposed in #121. That work can be evaluated separately against this read path and its benchmark tradeoffs. ## Validation - `corepack pnpm typecheck` - `corepack pnpm test` - 424 tests, coverage thresholds passed - `corepack pnpm build` - `corepack pnpm test:package` - including real node-redis and GLIDE standalone and Cluster consumer types, plus packed ESM/CommonJS absence checks for all four removed APIs - `corepack pnpm test:integration` - 113 tests across Redis 6.2, Valkey 8, and Redis Cluster - tracked wrong-type value repair and repeated wrong-type watermark fail-open behavior exercised end to end across both adapters and both standalone engines - stale tracked frames exercise the real decoder and record a remote miss, request/get/fallback timing, and no read error across both adapters and both standalone engines - fenced tracked writes prove stale-value unlinking while preserving the exact watermark and its TTL trajectory - cluster `SCRIPT FLUSH` recovery proves mutation scripts repopulate every master and a subsequent identical read is a cache hit - GLIDE package tests compile against the supported 2.0.0 floor and exercise separate module instances plus packed ESM/CommonJS error identity - focused GLIDE primary/replica probe and three-node Cluster probe - `git diff --check`
lan17
force-pushed
the
agent/stale-on-error
branch
from
August 19, 2026 23:35
a2f55ce to
388027c
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add opt-in stale-on-error recovery from a physically retained Redis value, reconciled onto the current v0.20 native-I/O, compression, coalescing, shadow, and GLIDE 2.x architecture.
F = ttlSec[CacheLayer.REMOTE]remains the logical fresh age.M = staleOnErrorMaxAgeSecis the absolute recovery age.0 <= age < F.0 <= age < M.Closes #117
Contract and flow
All source rejections qualify, including arbitrary rejection values and
FallbackTimeoutError. Recovery gets a fresh effectiveremoteReadTimeoutMsbudget. Existing coalescing shares the complete read/source/recovery chain;coalesce: falsepreserves independent chains. A recovered value may be memoized only inside the already-active request-local scope.Configuration
0: explicitly disables an inherited policy.0 < F < M <= 31,536,000seconds.Mrecordsconfig_resolution, disables only recovery for that invocation, and preserves an otherwise valid fresh Redis policy.DialCacheKeyConfig.disabled()explicitly setsMto0.The invocation's once-resolved
F/Msnapshot governs both reads. Lowering a boundary takes effect immediately. RaisingMcannot resurrect or extend a key written with a shorter physical TTL; only a later successful write gets the longer retention.Redis protocol and adapters
This keeps v0.20's native payload I/O; it does not restore the pre-v0.20 read Lua scripts.
Every semantic read enqueues an ordered same-primary pair in one pipeline/batch round trip:
GET, or atomic trackedMGET(value, watermark);TIMEon the same connection/primary.The adapter decodes the full
DecodedRedisFramein Node and accepts it only when0 <= serverNowMs - createdAtMs < maxAgeMs. Future, unsafe, malformed, unsupported, and watermark-fenced frames are misses or typed protocol errors according to the existing contract. Normal, shadow, and confirmation reads always passF, even when recovery is off; the post-source recovery reread alone passesM.Both write modes keep payload bytes out of Lua:
SET PXwrites a version-0 payload placeholder with a per-write nonce;The tracked stamp also fences the invalidation watermark and maintains its TTL. A lost or raced placeholder fails honestly instead of promoting another writer's frame. Node-redis and GLIDE preserve SET-error precedence and recover stamp scripts only after
NOSCRIPT.RedisReadRequest.maxAgeMsandDialCacheRedisClient.enforcesMaxAge: trueare required. The constructor rejects old/custom clients that do not attest to the logical-age contract. Shared protocol helpers and both stamp sources are exported throughdialcache/redis-protocol; packed TypeScript, ESM, and CommonJS fixtures cover them.Failure, invalidation, shadow, and publication safety
PX Mwhen opted in; all serving and shadow reads still enforceF.undefined, compression envelopes, request-local ownership, coalescing on/off, ramp-down shadowing, and future-stamped frame behavior have dedicated regressions.Observability
Add one optional bounded observer:
Outcomes are
served,miss,read_error,read_timeout, anddeserialization_error. Prometheus exposesdialcache_stale_recovery_counter; Datadog exposesdialcache.stale_recovery.count. Existing fallback errors and duration remain truthful even when retained data ultimately reaches the caller. Observer failures remain isolated.Compatibility and rollout
This intentionally reuses the existing frame-v1 key and is not compatible with old semantic Redis clients.
Roll out readers first:
Mremains omitted or0;F;Mfor selected use cases;An old reader relies on physical expiry and could serve retained
F..Mdata as fresh. Once any writer usesPX M, do not restore an old reader until the largest enabledMhas elapsed since the final such write, or the affected keys have been isolated/removed. Disabling recovery on new readers is safe because they continue enforcingF.Cost model and benchmark
GET/MGET+TIME).SET+ a header-only stamp script.The checked-in no-threshold benchmark asserts compression, physical TTL near
M, logical expiry atF, exact source/recovery counts, and coalesced fanout while reportingINFO commandstats, network deltas, and client throughput. One local Redis 8.8 / Node 22.22 run with a 64 KiB compressible JSON value observed:These loopback figures are directional, not production capacity promises; use
pnpm benchmark:stale-on-erroron the target engine and workload.Validation
corepack pnpm checkcorepack pnpm test:integrationpnpm benchmark:stale-on-error— semantic assertions passed against Redis 8.8pnpm benchmark:request-local— all 10 semantic scenarios passedgit diff --check